Fix stale MountAndActivate landmark in perf docs#696
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Removes leftover unguarded [embed:trace] Console.Error.WriteLine(...) printf-style debug logging from the window-open / first-frame activation path (including embed setup), keeping behavior and ordering unchanged while eliminating unconditional stderr noise.
Changes:
- Removed unconditional stderr trace lines from
ReactorApp.OpenWindowCoreand simplified no-op try/catch wrappers that only logged-then-rethrew. - Removed embed-only trace lines from
ReactorWindowembed setup and simplified the same log-only exception wrapper. - Updated the stress-perf CI troubleshooting note to no longer rely on the removed trace landmark.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Reactor/Hosting/ReactorApp.cs | Drops unconditional stderr traces in OpenWindowCore; preserves cleanup-on-activation-failure behavior. |
| src/Reactor/Hosting/ReactorWindow.cs | Removes embed setup stderr traces; keeps embed init ordering and watchdog start behavior intact. |
| tests/stress_perf/ci/README.md | Rewords troubleshooting guidance to avoid referencing removed trace output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
…t-frame path PR #543 (Spec 056 embedded preview Phase 1) left unguarded Console.Error.WriteLine("[embed:trace] ...") printf-debugging in the window-open path: four success-path writes fire on EVERY window open (even non-embed) plus catch-block traces, and six embed-only writes in ReactorWindow setup. Removed all of them while preserving behavior exactly: the MountAndActivate catch keeps its real cleanup (UnregisterWindow + Dispose) before rethrow; the ctor/configure and embed-setup try/catch blocks only logged-then-rethrew, so after removing the logs they are no-ops and were simplified away (exceptions propagate identically). No control-flow, ordering, or Register/Mount sequencing changes. Consumer check: the [embed:trace] lines go to stderr only; the VS preview handshake is CAPTURE_PORT/CAPTURE_TOKEN parsed from stdout (ReactorChildLauncher.cs, extension.ts). stderr is purely diagnostic (16KB tail buffer + Output pane), so nothing programmatic depends on these lines. Also updated a stale troubleshooting landmark in tests/stress_perf/ci/README.md that referenced the removed "MountAndActivate ok" trace. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…race The pr-review skill flagged that .github/skills/perf-compare/SKILL.md still referenced the removed "MountAndActivate ok" trace as a headless-crash landmark. Reworded to "during first-frame window activation" to match the README change so the skill no longer points readers at a stderr line that no longer prints. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
azchohfi
force-pushed
the
azchohfi-remove-embed-trace-logging
branch
from
0eeedc5 to
7a1dfe2
Compare
azchohfi
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updates two perf-tooling docs whose
0xC000027Btroubleshooting landmark still referenced the[embed:trace] … MountAndActivate oklog line that has since been removed from the window-open/first-frame path..github/skills/perf-compare/SKILL.mdtests/stress_perf/ci/README.mdBoth now describe the crash as happening "during first-frame window activation" instead of "right after MountAndActivate ok", so the guidance no longer points at a log line that no longer exists.